Codecov Report

Merging #781 into master will decrease coverage by <.01%.
The diff coverage is 98.33%.

@@            Coverage Diff             @@
##           master     #781      +/-   ##
==========================================
- Coverage   98.44%   98.44%   -0.01%     
==========================================
  Files         179      179              
  Lines        9951    10004      +53     
==========================================
+ Hits         9796     9848      +52     
- Misses        155      156       +1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec253f6...5fed948. Read the comment docs.

Hey @tinosulzer looks good. Couple things might want to consider. A dedicated notebook explaining the different ways to access the solution data. At the moment all examples get you to use quickplot but it's useful to know you can use solution as a dictionary directly and behind the scenes processed variables are created. The problem with this approach is that you can't do solution.keys() to get the options and solution.data.keys() only contains ones already processed so it's not obvious to the user what to do necessarily hence why an example might be useful. Another wrinkle with the functionality is that when stepping the solution if you want to access the data after each step it doesn't get automatically updated when the solutions re appended together. This is a fairly subtle thing and a experienced user who is stepping can probably work out that they need to manually call solution.update on the variable after appending. But worth knowing about so again could go in the example. I'm happy to write it and you can review it tomorrow to see if I got it all

@tinosulzer Feel free to change or move that new example. Also , do the notebooks get tested? If not I have a script that can do it on travis

Thanks @TomTranter , that example notebook is very clear. Note in the example you chose the stepped solution stops at the voltage cut-off while the full solution goes right through it (this is a bug in scipy) so it might be better to use a shorter t_eval to avoid confusion

Some ways I can think of getting around those issues you picked up on

• Write a method solution.keys() that returns model.variables.keys(). This is perhaps a bit dangerous as solution.values() and solution.items() would not act correspondingly. So perhaps safer to leave as-is and refer any questions to the example notebook.
• Add functionality to solution.append to automatically update any variables that have already been created. I had been meaning to do this anyway but forgot when I came back to this issue recently. I can't think of any downside with doing this?

Examples do get tested on travis and you can run locally with

python3 run-tests.py --examples

@tinosulzer I agree that solution.keys could get messy. It's not too much trouble to look in model.variables.keys. On the append solution that sounds good but would it add a lot of over head if the solution is appended to a lot. Might be better to leave that to. Up to you